fix(#210): replayed or cookie-less OAuth callbacks redirect home instead of 500 - #211
Open
sspickle wants to merge 1 commit into
Open
fix(#210): replayed or cookie-less OAuth callbacks redirect home instead of 500#211sspickle wants to merge 1 commit into
sspickle wants to merge 1 commit into
Conversation
… 500
The authlib state is single-use and lives in the session cookie, so two real
populations hit MismatchingStateError on /google/auth and got GAE's bare
'500 Server Error' page:
- anyone who REFRESHES the callback URL: the state was consumed on the first
attempt, so every retry can only fail. Observed live 2026-09-01: one
classroom machine retried a dead callback 68 times.
- browsers that refuse the session cookie, so no state is ever stored.
Production logs show a steady 1-2% of sign-ins failing this way for at least a
month (as far back as retention goes), across ~150/day summer traffic and
~900+/day semester-start traffic alike. Nothing changed server-side; the
semester surge just made a chronic failure loud.
Catch OAuthError (MismatchingStateError's base, which also covers a replayed
code's invalid_grant) and redirect to '/', where the user can simply sign in
again — the only action that can ever work. The bare /google/auth-with-no-state
branch ('Yikes!') now redirects too instead of falling through to a guaranteed
crash.
Tests reproduce the exact production exception (RED on master) and pin the
redirect. Suite run under python:3.12 (the GAE runtime): 21 passed, 2 failed —
both failures pre-exist on master in test_plotusers, unrelated.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #210.
What
/google/authcatchesOAuthError(the base ofMismatchingStateError, also covering a replayed code'sinvalid_grant) and redirects to/— the only place a user whose callback can never succeed can actually do something useful. The no-state branch ("Yikes!") now redirects too instead of falling through to a guaranteed crash.Why redirect rather than an error page
The state is single-use by design (CSRF protection — untouched here). Once it's consumed or the cookie never existed, retrying the same URL cannot work; the only recovery is starting a fresh sign-in from the landing page. A bare 500 invites exactly the wrong response — refresh — which is how one classroom machine produced 68 consecutive failures on 2026-09-01.
Testing
tests/test_auth_callback.pyreproduces the exact production exception (RED on master:MismatchingStateError: mismatching_state: CSRF Warning!) and pins the redirect for both the replay shape and the bare-URL shape.Suite run under
python:3.12(matching the GAE runtime, since a 3.13 venv can no longer import the app —cgiwas removed): 21 passed, 2 failed — both failures pre-exist on master intest_plotusersand are unrelated (verified by running master in the same container).Production data
Steady 1–2% of sign-ins have failed this way for as far back as retention reaches, across a July-15 deploy boundary and a 6× traffic change — full numbers in #210.